Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[14.0][IMP] mail_tracking: add job to delete old tracking records #1430

Merged

Conversation

henrybackman
Copy link
Contributor

@henrybackman henrybackman commented Aug 8, 2024

Records in mail_tracking_email table grow over time and increase the database size, which slows down backup and restore operations. (For example, in one customer db the mail_tracking_email table takes almost 2GB)

Added autovacuum to mail_tracking_email that removes by default records over 6 months old.
The deletion can be enabled with a configuration variable.
Due to possibly a large number of records to be deleted on first run, set a default limit of 5000 per run.

@henrybackman henrybackman force-pushed the 14.0-mail-tracking-email-deletion-job branch from 37b4f95 to 4e36e51 Compare August 8, 2024 08:44
@pedrobaeza
Copy link
Member

Do you have some metrics about performance degradation that leads to this action?

@sebalix
Copy link
Contributor

sebalix commented Aug 8, 2024

@pedrobaeza this action is here to reduce the DB size (to speed up backup and restore operations mainly), currently we have a DB with the mail_tracking_value taking almost 2GB (in the top 10 of biggest table in this DB, 8th position, 1st being mail_message).
About performance I checked but there isn't a real impact here (while this similar PR about notifications is reducing a lot the time needed to open the Odoo JS client: #1340)

@pedrobaeza
Copy link
Member

OK, thanks for the information. It's good to add this information initially (and include it in the commit message) for avoiding to guess what is the improvement.

It would be interesting to check if there's a way to optimize the DB schema of this table to reduce this size as well. Maybe there's a related stored or similar fields that increase the size a lot.

@henrybackman henrybackman force-pushed the 14.0-mail-tracking-email-deletion-job branch from 34e286e to 36b36cf Compare August 8, 2024 11:48
@henrybackman
Copy link
Contributor Author

Thank you @pedrobaeza for checking. I don't have anything to add to @sebalix comment but I added the relevant details to the description 👍

@henrybackman henrybackman force-pushed the 14.0-mail-tracking-email-deletion-job branch from 36b36cf to dc9ef3c Compare August 8, 2024 11:56
@henrybackman henrybackman force-pushed the 14.0-mail-tracking-email-deletion-job branch from dc9ef3c to 7e95078 Compare August 9, 2024 11:18
@henrybackman
Copy link
Contributor Author

Test added to the tests/__init__.py and improved wording in the settings description

@simahawk
Copy link
Contributor

simahawk commented Aug 9, 2024

@henrybackman when you push again pls report the description in the commit msg.
Note: if you do that before opening a PR you'll get the PR description for free 😉

@henrybackman henrybackman force-pushed the 14.0-mail-tracking-email-deletion-job branch from 7e95078 to 3fa48e9 Compare August 9, 2024 13:51
Comment on lines 496 to 498
records_to_delete = self.search(domain, limit=limit)
_logger.debug("Deleting %s mail.tracking.email records", len(records_to_delete))
return records_to_delete.unlink()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After testing this on a customer DB, it triggers the following error (linked to related fields):

Traceback (most recent call last):
  File "/odoo/src/odoo/addons/base/models/ir_autovacuum.py", line 38, in _run_vacuum_cleaner
    func(model)
  File "/odoo/external-src/social/mail_tracking/models/mail_tracking_email.py", line 503, in _gc_mail_tracking_email
    records_to_delete.unlink()
  File "/odoo/external-src/connector/component_event/models/base.py", line 118, in unlink
    result = super(Base, self).unlink()
  File "/odoo/src/odoo/models.py", line 3501, in unlink
    self.flush()
  File "/odoo/src/odoo/models.py", line 5472, in flush
    self.recompute()
  File "/odoo/src/odoo/models.py", line 5931, in recompute
    process(field)
  File "/odoo/src/odoo/models.py", line 5918, in process
    field.recompute(existing)
  File "/odoo/src/odoo/fields.py", line 1178, in recompute
    self.compute_value(record)
  File "/odoo/src/odoo/fields.py", line 1198, in compute_value
    records._compute_field_value(self)
  File "/odoo/src/odoo/models.py", line 4079, in _compute_field_value
    odoo.fields.determine(field.compute, self)
  File "/odoo/src/odoo/fields.py", line 85, in determine
    return needle(records, *args)
  File "/odoo/src/odoo/fields.py", line 574, in _compute_related
    record[self.name] = self._process_related(value[self.related_field.name])
  File "/odoo/src/odoo/models.py", line 5702, in __getitem__
    return self._fields[key].__get__(self, type(self))
  File "/odoo/src/odoo/fields.py", line 2495, in __get__
    return super().__get__(records, owner)
  File "/odoo/src/odoo/fields.py", line 1025, in __get__
    _("(Record: %s, User: %s)") % (record, env.uid),
odoo.exceptions.MissingError: Record does not exist or has been deleted.

I tried different things but only a raw DELETE query is working while being simple:

Suggested change
records_to_delete = self.search(domain, limit=limit)
_logger.debug("Deleting %s mail.tracking.email records", len(records_to_delete))
return records_to_delete.unlink()
records_to_delete = self.search(domain, limit=limit).exists()
if records_to_delete:
_logger.info("Deleting %s mail.tracking.email records", len(records_to_delete))
self.flush()
query = "DELETE FROM mail_tracking_email WHERE id IN %s"
args = (tuple(records_to_delete.ids),)
self.env.cr.execute(query, args)
self.invalidate_cache()

@henrybackman henrybackman force-pushed the 14.0-mail-tracking-email-deletion-job branch 3 times, most recently from 722eeeb to 6ed27f3 Compare August 14, 2024 11:04
Add autovacuum to mail_tracking_email that removes old records based on new configuration variable mail_tracking_email_max_age_days.
Due to possibly a large number of records to be deleted on first run, set a default limit of 5000 per run.
@henrybackman henrybackman force-pushed the 14.0-mail-tracking-email-deletion-job branch from 6ed27f3 to 9fd6204 Compare August 20, 2024 08:35
@simahawk
Copy link
Contributor

@pedrobaeza good for you?

@pedrobaeza pedrobaeza added this to the 14.0 milestone Aug 26, 2024
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-1430-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 6bfbb1c into OCA:14.0 Aug 26, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 3bd12c2. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants